Skip to content

Conversation

@dhaval24
Copy link
Contributor

@dhaval24 dhaval24 commented Dec 13, 2018

Final W3C implementation

#775 #782 #776 #716

@lmolkova you can skip this review if you want as you already reviewed the old PR.
@littleaj can you take a brief overview?

Enable W3C:

Incoming Side -

J2EE Apps add the following to the <TelemetryModules> tag inside ApplicationInsights.xml

<Add type="com.microsoft.applicationinsights.web.extensibility.modules.WebRequestTrackingTelemetryModul>
   <Param name = " W3CEnabled" value ="true"/>
   <Param name ="enableW3CBackCompat" value = "true" />

</Add>

Springboot apps add the following property:
azure.application-insights.web.enable-W3C=true
azure.application-insights.web.enable-W3C-backcompat-mode=true

Outgoing Side -

Add the following to AI-Agent.xml

<Instrumentation>
        <BuiltIn enabled="true">
            <HTTP enabled="true" W3C="true" enableW3CBackCompat="true"/>
        </BuiltIn>
    </Instrumentation>

Note: Please note that Backward compatibility mode is enabled by default and the enableW3CBackCompat parameter is optional and should be used only when you want to turn it off.

Ideally this would be the case when all your services have been updated to newer version of SDKs supporting W3C protocol. It is highly recommended to move to newer version of SDKs with W3C support as soon as possible.

Please ensure that both incoming and outgoing configurations are exactly same, or else it's very likely that correlation won't work properly.
i.e W3C and bckportMode on on incoming side -> Then W3C and backport mode should on on outgoing side as well.

For significant contributions please make sure you have completed the following items:

  • Design discussion issue #
  • Changes in public surface reviewed
  • CHANGELOG.md updated

dhaval24 and others added 6 commits November 28, 2018 13:57
* W3C Tracecontext

* adding comments

* addressing PR comments
* Initial integration of W3C protocol for incoming request

* refactoring header name

* updating the verbosity level when correlation fails

* Fix bugs in correlation found via tests, implement more unit tests and address partial PR comments

* improve createOutboundTracestate()

* create tracestate when header not available

* fix test

* enable outbout w3c

* Refactering code to use Helper methods with TraceContext classes

*  fix tracestate integration, fix outbound tracestate injection, fix tests, propogate traceflags

* add property to turn on W3C in springboot starter, remove debug logs

* adopt internal storage of id's to legacy AI format for backport, update tests

* address PR comments

* Fix an incorrect assert

* refactor resolveCorrelation() method to be more readable and debuggable

* rename method names, create outbound traceparent for http if there is no incoming request too

* fixing a bug in w3c config for agent

* fix the dependency type name, fix target to be host+port | target
@dhaval24
Copy link
Contributor Author

image

E2E correlation works. W3CTestDotNetApp has W3C turned off and only emits request-ids.

@dhaval24
Copy link
Contributor Author

@lmolkova thanks for your comments. I have address them.

@dhaval24
Copy link
Contributor Author

@lmolkova I have now added extensive tests to cover all the possible scenarios. I believe all the concerns are resolved and we are ready to merge.
@littleaj if you don't have question would you mind taking a quick glance and approving this.

Copy link

@lmolkova lmolkova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dhaval24
Copy link
Contributor Author

#788

@trask trask deleted the W3CImplementation branch September 21, 2020 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants